Skip to content

[rtl] FPGA bkdr fix#30680

Merged
vogelpi merged 1 commit into
lowRISC:masterfrom
gautschimi:fpga_bkdr_fix
Jul 13, 2026
Merged

[rtl] FPGA bkdr fix#30680
vogelpi merged 1 commit into
lowRISC:masterfrom
gautschimi:fpga_bkdr_fix

Conversation

@gautschimi

@gautschimi gautschimi commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

The enum was previously defined as
typedef enum logic [$clog2(NumBkdrTgts)-1:0] and NumBkdrTgts was 12. Hence, the enum was 4bit wide.

The index check (https://github.com/lowRISC/opentitan/blob/master/hw/ip/bkdr_loader/rtl/bkdr_loader.sv#L284)
assign tgt_idx_err = !(bkdr_idx_e'(reg2hw.control.target_idx.q) inside {BkdrValidTgts});

It first casts the 8b target_idx signal to the enum size (dropping 4 bits). And then checks if the value is inside the valid targets. Hence, a target_idx of 13 was seen as a legal value. Actually vivado was interpreting it as always false and was thus always reporting an invalid target.

@gautschimi gautschimi changed the title Fpga bkdr fix [rtl] FPGA bkdr fix Jul 7, 2026
@gautschimi gautschimi force-pushed the fpga_bkdr_fix branch 4 times, most recently from f643fc1 to 8fe0fb5 Compare July 10, 2026 06:55
@gautschimi gautschimi marked this pull request as ready for review July 10, 2026 07:01
@gautschimi

Copy link
Copy Markdown
Contributor Author

The first commit of this PR was already merged as part of #30543

@gautschimi gautschimi requested a review from vogelpi July 10, 2026 07:06
@gautschimi gautschimi self-assigned this Jul 10, 2026
Comment thread hw/ip/bkdr_loader/rtl/bkdr_loader_pkg.sv Outdated

@a-will a-will left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand why this enum even exists. It looks like this became a top-specific IP instead of something generic.

This should probably become templated if the CSR layout is so tied to a specific top.

tgt_idx_err was not set when the index exceeded NumBkdrTgts because it
was casted to a logic with fewer bits. The enum is now changed to an 8b
enum such that the cast is not dropping any bits.

Signed-off-by: Michael Gautschi <mgautschi@lowrisc.org>

@vogelpi vogelpi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM!

@vogelpi

vogelpi commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

The e2e_bootstrap_disabled_fpga_cw340_rom_with_fake_keys FPGA test failed, but I've seen this failing for other PRs as well. The failure is not related to this change.

@vogelpi vogelpi added this pull request to the merge queue Jul 13, 2026
Merged via the queue into lowRISC:master with commit bdc0a81 Jul 13, 2026
42 of 43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants